-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: add CometAPI as new model provider #7689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add CometAPI type definitions with support for GPT-5, Claude-4, Gemini-2.5, and other models - Implement CometAPIHandler extending RouterProvider for OpenAI-compatible API - Add CometAPI to provider settings and configuration - Update webview components to support CometAPI provider - Add CometAPI to dynamic providers list for model fetching Implements #7688
- Remove duplicate schema entry in provider-settings.ts - Remove try-catch from fetchModel to match DeepInfraHandler pattern - Add timeout support to RouterProvider base class - Store apiKey and baseURL in RouterProvider for model fetching - Add comprehensive test coverage for CometAPI provider - Fix test mock structure for streaming responses
| _metadata?: ApiHandlerCreateMessageMetadata, | ||
| ): ApiStream { | ||
| // Ensure we have up-to-date model metadata | ||
| await this.fetchModel() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid duplicate calls to fetchModel() in createMessage. The method calls await this.fetchModel() twice consecutively (lines 64 and 66). Consider storing the result of the first call in a variable to prevent redundant network requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| ): ApiStream { | ||
| // Ensure we have up-to-date model metadata | ||
| await this.fetchModel() | ||
| const { id: modelId, info, reasoningEffort: reasoning_effort } = await this.fetchModel() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? You're calling fetchModel() twice here - once on line 65 and again on line 66. This could cause duplicate API calls and impact performance. Consider removing the duplicate call:
| public override async fetchModel() { | ||
| this.models = await getModels({ | ||
| provider: this.name, | ||
| apiKey: this.apiKey, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we improve consistency here? The CometAPI handler uses this.apiKey and this.baseURL directly, while DeepInfra uses this.client.apiKey and this.client.baseURL. Since RouterProvider now exposes these as protected properties, should we standardize on one approach across all router providers?
| systemPrompt: string, | ||
| messages: Anthropic.Messages.MessageParam[], | ||
| _metadata?: ApiHandlerCreateMessageMetadata, | ||
| ): ApiStream { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing prompt caching support - several CometAPI models claim to support prompt caching, but the implementation doesn't handle it. DeepInfra shows how to implement this with prompt_cache_key. Should we add this for better performance?
| ...options, | ||
| openAiHeaders: { | ||
| "X-CometAPI-Source": "roo-code", | ||
| "X-CometAPI-Version": `2025-09-05`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is hardcoding the version date the best approach? This '2025-09-05' will become outdated quickly. Consider using a package version or removing it entirely to avoid maintenance burden.
| contextWindow: 128000, | ||
| supportsImages: true, | ||
| supportsPromptCache: false, | ||
| inputPrice: 2.5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these pricing values accurate? GPT-5 at .5/0 seems like it might be a placeholder. Should we fetch pricing dynamically from the API to ensure accuracy, or at least add a comment indicating these need verification?
| const handler = new CometAPIHandler(options) | ||
|
|
||
| // Should not throw, error is handled by getModels which returns fallback models | ||
| await expect(handler.fetchModel()).rejects.toThrow("Network error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add more error scenario tests? While the happy path is well covered, consider adding tests for:
- API failures during model fetching
- Invalid/malformed API responses
- Network timeouts
- Edge cases in model selection
This would improve confidence in the error handling.
|
Closing in favor of #7708 |
Fixes #7688
Summary
This PR adds CometAPI as a new model provider to Roo Code, giving users access to cutting-edge AI models including GPT-5, Claude-4, Gemini-2.5, and others through a unified OpenAI-compatible API.
Changes Made
Core Implementation
Model Support
Features
Testing
Code Quality
Testing Instructions
Build the extension:
pnpm buildAdd CometAPI configuration in settings:
Test model selection and chat functionality
Verify streaming responses work correctly
Test with different model types (standard, reasoning, vision)
Checklist
Screenshots
N/A - Backend provider implementation
Related Issues
Fixes #7688
Important
Add CometAPI as a new model provider with comprehensive integration and testing.
CometAPIHandlerextendingRouterProviderfor OpenAI-compatible API incometapi.ts.cometApiSchematoprovider-settings.tsfor configuration.buildApiHandler()inindex.tsto includeCometAPIHandler.COMETAPI_MODELSandcometApiDefaultModelInfoincometapi.ts.getCometAPIModels()infetchers/cometapi.tsfor model fetching.CometAPIHandlerin__tests__/cometapi.spec.ts.getApiRequestTimeoutandgetModelsfor testing.useSelectedModel.tsto support CometAPI model selection.cometapitodynamicProvidersinprovider-settings.ts.This description was created by
for aa7ed61. You can customize this summary. It will automatically update as commits are pushed.